Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove support for pre v16 joi #4474

Open
wants to merge 1 commit into
base: next
Choose a base branch
from
Open

Conversation

kanongil
Copy link
Contributor

@kanongil kanongil commented Nov 17, 2023

The last [email protected] was released more than 4 years ago. It should be safe to drop (in a breaking release).

This also removes several npm deprecation notices when installing dev dependencies.

@kanongil kanongil added the breaking changes Change that can breaking existing code label Nov 17, 2023
@damusix
Copy link
Contributor

damusix commented Nov 28, 2024

@kanongil Is this still a valid PR? Any chance you can update it if need be? We're converging for a major release and may include this if it's ready.

@kanongil
Copy link
Contributor Author

There might be an issue with this PR, let me check…

Copy link
Contributor Author

@kanongil kanongil left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to keep support for non-async validators, since an external validator might only use this.

Ie. I have removed the logic changes, and this patch just replaces the legacy joi test, with a custom non-async validator test.

The test failures seem unrelated to this PR. Note that node@14 also a fails while reporting a false positive.

@kanongil kanongil added test Test or coverage and removed breaking changes Change that can breaking existing code labels Nov 29, 2024
@kanongil
Copy link
Contributor Author

Note that since this no longer changes the logic, this patch could also be applied against the main branch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
test Test or coverage
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants